Skip to content

feat: add PTY tools for background terminal sessions#6

Merged
vtemian merged 15 commits intomainfrom
feature/pty-integration
Jan 3, 2026
Merged

feat: add PTY tools for background terminal sessions#6
vtemian merged 15 commits intomainfrom
feature/pty-integration

Conversation

@vtemian
Copy link
Owner

@vtemian vtemian commented Jan 3, 2026

Summary

  • Port opencode-pty functionality into our codebase
  • Add 5 PTY tools for managing background terminal sessions
  • Update agent prompts to use PTY tools intelligently

New Tools

Tool Purpose
pty_spawn Start a background terminal session (dev servers, watch modes, REPLs)
pty_write Send input to a PTY (text, Ctrl+C, etc.)
pty_read Read output buffer with pagination and regex filtering
pty_list List all PTY sessions with status
pty_kill Terminate a PTY session

Key Distinction

System Purpose Example
background_task Async AI subagents Research, implementation, reviews
pty_spawn Async bash processes Dev servers, watch modes, REPLs

Changes

New Files (17)

  • src/tools/pty/ - 9 files (types, buffer, manager, 5 tools, index)
  • tests/tools/pty/ - 7 test files (46 tests)

Modified Files

  • package.json - add bun-pty dependency
  • src/index.ts - register PTY tools with session cleanup
  • Agent prompts (commander, implementer, executor, reviewer) - add PTY guidance

Testing

  • 153 tests passing (46 new PTY tests)
  • TypeScript compiles cleanly
  • Build succeeds

Design Document

See thoughts/shared/designs/2026-01-01-pty-integration-design.md


Summary by cubic

Adds PTY tools to manage background terminal sessions so long-running and interactive processes don’t block the agent. Prompts now choose between bash and PTY, and sessions auto-clean on session deletion.

  • New Features

    • PTYManager + RingBuffer with paginated output and regex search; tools: pty_spawn, pty_write, pty_read, pty_list, pty_kill.
    • Agent prompts updated with clear bash vs PTY rules; executor and reviewer include PTY usage and cleanup checks.
    • PTY sessions tie to the parent session and are cleaned up on session.deleted events.
  • Bug Fixes

    • Auto-clear ledger threshold unified to 60%.
    • pty_read normalizes negative offsets; clearer invalid regex errors.
    • pty_write supports null byte (\0) and reports accurate byte counts.
    • pty_spawn: description argument is optional.

Written for commit aa4b550. Summary will update on new commits.

- Add RingBuffer for output capture with search support
- Add PTYManager for session lifecycle management
- Add pty_spawn, pty_write, pty_read, pty_list, pty_kill tools
- Add comprehensive test coverage for all components
- Commander: terminal tool selection guidance
- Implementer: bash vs pty decision rules
- Executor: PTY tools for background processes
- Reviewer: PTY cleanup verification
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 issues found across 23 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/tools/pty/tools/write.ts">

<violation number="1" location="src/tools/pty/tools/write.ts:92">
P2: The return message reports `args.data.length` bytes but actually sends `parsedData.length` bytes. After escape sequence parsing, `\x03` (4 chars) becomes a single byte, making the reported count inaccurate. Use `parsedData.length` for the correct byte count.</violation>
</file>

<file name="src/index.ts">

<violation number="1" location="src/index.ts:242">
P2: Duplicated `session.deleted` check. This new block is identical in structure to the existing one above it. Consider merging the PTY cleanup into the existing `session.deleted` handler to reduce code duplication and improve maintainability.</violation>
</file>

<file name="tests/tools/pty/read.test.ts">

<violation number="1" location="tests/tools/pty/read.test.ts:74">
P2: Weak assertion - this only verifies the output format includes `pattern=`, not that filtering actually works. Consider asserting that the result contains the matched line (&quot;error: bad&quot;) and/or doesn&#39;t contain non-matching lines (&quot;line1&quot;, &quot;line3&quot;).</violation>
</file>

<file name="src/tools/pty/tools/spawn.ts">

<violation number="1" location="src/tools/pty/tools/spawn.ts:50">
P2: The `description` parameter is required but never used. It&#39;s not passed to `manager.spawn()`, not stored in the session, and not included in the output. Either pass the description through the manager to store it (requires updating `SpawnOptions`, `PTYSession`, and `PTYSessionInfo` types), or mark it as `.optional()` if it&#39;s not needed.</violation>
</file>

<file name="tests/tools/pty/spawn.test.ts">

<violation number="1" location="tests/tools/pty/spawn.test.ts:49">
P2: Potential flaky test: `echo hello` completes almost instantly, so asserting `Status: running` may fail intermittently due to a race condition. Consider using a longer-running command like `sleep 1` or `cat`, or assert on a status that accommodates both running and completed states.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

- buffer.test.ts: negative offset, empty string, unicode handling
- manager.test.ts: read() with offset/limit, search(), write to killed session
- integration.test.ts: spawn → write → read → kill lifecycle tests
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 7 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="tests/tools/pty/manager.test.ts">

<violation number="1" location="tests/tools/pty/manager.test.ts:146">
P2: Weak assertion: `toBeGreaterThanOrEqual(0)` will always pass since array length can never be negative. This test named &quot;should find matching lines&quot; doesn&#39;t actually verify that matches were found. Use `toBeGreaterThanOrEqual(1)` or `toBeGreaterThan(0)` to validate the search functionality.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

- write.ts: Report parsedData.length instead of args.data.length for accurate byte count
- spawn.ts: Make description parameter optional (was required but unused)
- read.test.ts: Strengthen pattern filtering assertion to verify actual filtering
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/tools/pty/tools/spawn.ts">

<violation number="1" location="src/tools/pty/tools/spawn.ts:51">
P2: Documentation/schema mismatch: The `DESCRIPTION` constant states the `description` parameter is required, but the schema now marks it as optional. Either update the documentation to reflect that description is optional, or remove `.optional()` if it should remain required.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

- spawn.test.ts: Use sleep instead of echo to avoid race condition on status check
- manager.test.ts: Use toBeGreaterThan(0) instead of toBeGreaterThanOrEqual(0) for meaningful assertion
- spawn.ts: Fix documentation to reflect that description is optional
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 3 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="README.md">

<violation number="1" location="README.md:13">
P2: JSON does not support comments. Users copying this config block directly will get a parse error. Consider moving the path comment outside the code block or using a JSON5 syntax hint.</violation>

<violation number="2" location="README.md:94">
P2: Incorrect threshold value. The `auto-clear-ledger` hook uses 80% threshold (see `src/hooks/auto-clear-ledger.ts`), not 60%. The 60% threshold is for `preemptive-compaction`, which is a different hook.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

- Remove JSON comment (not valid JSON)
- Clarify threshold: preemptive compaction at 60%, auto-clear at 80%
- Update auto-clear-ledger from 80% to 60%
- Update test expectations
- Update README to reflect consistent 60% threshold
@vtemian vtemian merged commit 70c247f into main Jan 3, 2026
2 checks passed
@vtemian vtemian deleted the feature/pty-integration branch January 3, 2026 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant